Resolve additional test failures for c2 branch#21656
Merged
Merged
Conversation
adfoster-r7
force-pushed
the
patch-additional-c2-errors
branch
3 times, most recently
from
8955f11 to
5a185c0
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses additional test failures on the C2 branch by adjusting acceptance expectations, expanding payload cache-size consistency coverage, and applying a few payload/CI fixes aimed at stabilizing Meterpreter-related tests across platforms.
Changes:
- Expanded
spec/modules/payloads_spec.rbto cover cached-size consistency for additional Java and PHP Meterpreter payloads. - Updated PHP Meterpreter single payloads to include
Msf::Sessions::MeterpreterOptions::Php(language-specific options module). - Adjusted CI workflows to support building against a configurable
metasploit-payloadsrepository/commit (but currently introduces hardcoded fork/branch usage inmeterpreter_acceptance.yml).
Impact Analysis:
- Blast radius: medium — affects CI signal for Meterpreter acceptance, and alters behavior in shared payload code paths (
MachO#sign) used by macOS payload generation. - Data and contract effects: low — no schema changes; however, CI contract/inputs behavior is impacted by the hardcoded repo/commit in
meterpreter_acceptance.yml. - Rollback and test focus: rollback is straightforward (workflow + small library/module changes); prioritize validating Meterpreter acceptance workflows (repo/commit selection + build gating) and macOS Mach-O signing correctness on Linux runners.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
spec/support/acceptance/session/php.rb |
Removes an OSX known-failure entry for a PHP session acceptance case. |
spec/modules/payloads_spec.rb |
Adds cached-size consistency specs for additional Java and PHP Meterpreter payloads. |
modules/payloads/singles/php/meterpreter_reverse_http.rb |
Switches to PHP-specific Meterpreter options mixin. |
modules/payloads/singles/php/meterpreter_reverse_https.rb |
Switches to PHP-specific Meterpreter options mixin. |
lib/rex/proto/http/server.rb |
Comment-only ASCII punctuation change. |
lib/rex/proto/http/request.rb |
Comment-only change (removes non-ASCII shrug). |
lib/msf/core/payload/malleable_c2.rb |
Comment-only ASCII punctuation changes. |
lib/msf/core/payload/macho.rb |
Preserves original Mach-O bytes and uses them for signing to avoid ruby-macho serialize issues on Linux. |
lib/msf/core/payload/java/meterpreter_loader.rb |
Comment-only ASCII punctuation changes. |
lib/msf/core/handler/reverse_http.rb |
Comment-only ASCII punctuation changes. |
lib/msf/base/sessions/mettle_config.rb |
Comment-only ASCII punctuation change. |
.github/workflows/shared_meterpreter_acceptance.yml |
Adds an input to select the metasploit-payloads repository and tweaks job naming/comments. |
.github/workflows/meterpreter_acceptance.yml |
Changes defaults and (currently) hardcodes a fork/branch + forces builds (needs correction). |
.github/workflows/command_shell_acceptance.yml |
Updates default metasploit-payloads commit to 6.5. |
Comments suppressed due to low confidence (1)
.github/workflows/meterpreter_acceptance.yml:69
- Critical: Problem: The meterpreter_acceptance workflow hardcodes a fork/branch for metasploit-payloads and forces payload/mettle builds on every run, ignoring workflow_dispatch inputs and PR labels. Impact: CI will run against non-upstream payload sources and do extra compilation work, making results unreliable and increasing runtime/cost. Fix: Pass through the workflow inputs and restore label-gated build flags (and rely on shared workflow defaults for the repo).
build:
uses: ./.github/workflows/shared_meterpreter_acceptance.yml
with:
metasploit_payloads_repo: adfoster-r7/metasploit-payloads
metasploit_payloads_commit: update-php-meterpreter-errors
mettle_commit: ${{ github.event.inputs.mettle_commit || '6.5' }}
build_metasploit_payloads: true
build_mettle: true
78333fd to
e70625c
Compare
adfoster-r7
force-pushed
the
patch-additional-c2-errors
branch
4 times, most recently
from
04ae89e to
af16317
Compare
7 tasks
adfoster-r7
force-pushed
the
patch-additional-c2-errors
branch
5 times, most recently
from
e016a13 to
d5604fe
Compare
adfoster-r7
commented
Jul 8, 2026
| known_failures: [ | ||
| "[-] FAILED: should return a list of processes" | ||
| ] | ||
| known_failures: [] |
Contributor
Author
There was a problem hiding this comment.
Resolved: rapid7/metasploit-payloads#802
adfoster-r7
commented
Jul 8, 2026
| @@ -109,19 +109,22 @@ jobs: | |||
| tools: none | |||
|
|
|||
| - name: Install system dependencies (Windows) | |||
Contributor
Author
There was a problem hiding this comment.
This was silently broken when pointing towards the custom 6.5 branches
adfoster-r7
commented
Jul 8, 2026
| shell: cmd | ||
| shell: pwsh | ||
| if: runner.os == 'Windows' | ||
| run: | |
Contributor
Author
There was a problem hiding this comment.
This was silently broken when pointing towards the custom 6.5 branches
adfoster-r7
commented
Jul 8, 2026
| # block. Catch the overflow here with a useful message instead of | ||
| # letting the gem's `to_binary` raise a generic "config block too | ||
| # large" — baked-in EXTENSIONS= is the usual culprit. | ||
| # large" -- baked-in EXTENSIONS= is the usual culprit. |
Contributor
Author
There was a problem hiding this comment.
Getting rid of non-ascii chars which aren't allowed in modules
adfoster-r7
force-pushed
the
patch-additional-c2-errors
branch
2 times, most recently
from
3060458 to
d5f54a8
Compare
adfoster-r7
force-pushed
the
patch-additional-c2-errors
branch
7 times, most recently
from
610a280 to
e569df0
Compare
adfoster-r7
force-pushed
the
patch-additional-c2-errors
branch
from
e569df0 to
082d99f
Compare
Contributor
Author
|
Looks like windows 2022 has been failing since pre5, but passed on pre4 - will merge this in, and resolve/debug that separately |
adfoster-r7
commented
Jul 9, 2026
| metasploit-credential (>= 6.0.21) | ||
| metasploit-model | ||
| metasploit-payloads (= 2.0.245) | ||
| metasploit-payloads (= 2.0.246.pre.7) |
Contributor
Author
There was a problem hiding this comment.
Looks like windows 2022 has been failing since pre5, but passed on pre4 - will merge this in, and resolve/debug that separately
cgranleese-r7
approved these changes
Jul 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Patching additional c2 errors